Fix JSON reexport ICE #97599
Fix JSON reexport ICE #97599
Conversation
GuillaumeGomez
added
T-rustdoc
rust-highfive
added
the
S-waiting-on-review
|
This is more like a bandage than a real solution. You should look at #93518 which is a completely different approach than this PR is taking, basically the idea is for the JSON output to not inline any item like the html output is doing, meaning that re-exports will not ICE anymore because they will still be different and thus not have the same id. |
|
Does that PR fix this bug? If so, then I'd like to merge that one instead. |
|
That PR needs updated and is not yet perfect, it still has some known failures. On the other hand as far as I can tell it does fix re-export issues generally. I've been slacking on doing the work to carry it over the finish-line, I'd be willing to help mentor someone else if they are willing to do some of the work to push it over the finish-line, alternatively I'll try to set aside some time this weekend to finish it up. |
|
I also took another approach at the start where I stopped inlining reexports but I found the result sub-optimal. However it'd be a better approach overall than my current PR. What is missing in #93518? |
It's currently blocked on fixing #93518 (comment) and applying this suggestion #93518 (comment). And maybe adding some more tests cases. |
|
I see. Considering none of these PRs are anywhere close to being merged, should we move forward with this one then? I can add a My problem here is that we have an ICE, which should be avoided at all cost. |
|
I'd support moving forward with this as a future FIXME |
|
Ok, updating the PR then. :) |
GuillaumeGomez
force-pushed
the
reexport-json
branch
from
43bcbfb to
069ae16
Compare
|
And done! |
|
@bors: r=CraftSpider rollup |
|
📌 Commit 069ae16 has been approved by |
bors
added
S-waiting-on-bors
…ftSpider Fix JSON reexport ICE Fixes rust-lang#97432. The problem was that the ID was conflicting because the reexports have the same one. To fix it, I "extended" it by adding the `Symbol` into it as well. r? `@notriddle`
|
failed in rollup |
|
@bors r- |
bors
added
S-waiting-on-author
GuillaumeGomez
force-pushed
the
reexport-json
branch
from
069ae16 to
5adca73
Compare
|
Fixed the doc comment. @bors: r=CraftSpider rollup |
|
📌 Commit 5adca73 has been approved by |
bors
added
S-waiting-on-bors
Rollup of 5 pull requests Successful merges: - rust-lang#97420 (Be a little nicer with casts when formatting `fn` pointers) - rust-lang#97450 ([RFC 2011] Basic compiler infrastructure) - rust-lang#97599 (Fix JSON reexport ICE) - rust-lang#97617 (Rustdoc anonymous reexports) - rust-lang#97636 (Revert rust-lang#96682.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #97432.
The problem was that the ID was conflicting because the reexports have the same one. To fix it, I "extended" it by adding the
Symbolinto it as well.r? @notriddle